-
Notifications
You must be signed in to change notification settings - Fork 8
Immutable Tuples as C Structs, not pointers #248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Turn immutable tuples into C structs, and keep mutable tuples as C pointers.
|
While doing this PR, I have been stomping out other bugs, including stuff from my own codegen tests import numpy as np
import numpy.ctypeslib as ctl
a = np.int64(2)
b = np.int64(1)
s1 = ctl.as_ctypes(a)
s2 = ctl.as_ctypes(b)
print(s1.value, s2.value)
# this prints 1 1 :skull: |
willow-ahrens
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I would only ask for an ImmutableFType class-based interface.
Even though literally everything should be immutable right now
src/finchlite/codegen/c.py
Outdated
|
|
||
| def scalar_to_ctypes_copy(fmt, obj): | ||
| """ | ||
| This hack is required because it turns out that scalars don't own memory or smth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reported it here: numpy/numpy#30354
src/finchlite/codegen/c.py
Outdated
|
|
||
| import numpy as np | ||
|
|
||
| from finchlite.finch_assembly.struct import MutableStructFType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: Let's use one convention, with relative import as below.
| np.generic, | ||
| "serialize_to_c", | ||
| "__attr__", | ||
| lambda fmt, obj: np.ctypeslib.as_ctypes(obj), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have it as np.ctypeslib.as_ctypes(np.array(obj)), instead? It looks that 0-d arrays print your example correctly. Then we shouldn't need scalar_to_ctypes_copy.
Turn immutable tuples into C structs, and keep mutable tuples as C pointers.